Validate warehouse location name length instead of silently truncating it - #834
Merged
Conversation
…g it Creating/editing a warehouse location let users type a name longer than the DB column, which MySQL then silently truncated on save with no feedback. Widens locations.label to varchar(50) and adds matching client + server validation so an over-limit name is rejected with a clear inline error (plus a tooltip explaining the limit) instead of being cut off. Also fixes two latent bugs this surfaced: the data-max-count character counter's jQuery plugin was never actually loaded on any page, and its counter text was hardcoded in Dutch with an unused English translate key sitting right next to it.
Contributor
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents warehouse location names from being silently truncated by aligning database limits with explicit validation and UI feedback in the CMS “Edit Warehouses” flow.
Changes:
- Widen
locations.labelfromvarchar(20)tovarchar(50)via a new Phinx migration. - Add server-side length validation in
include/locations_edit.phpand client-sidemaxlengthenforcement fordata-max-countfields via jQuery Validate. - Ensure the character counter works end-to-end by loading
jquery.simplyCountable.jsand using the existing translate key for “characters left”.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| templates/cms_form_url.tpl | Removes an unused/legacy field template variant. |
| templates/cms_form_text.tpl | Switches counter label text to use the cms_form_charactersleft translate key. |
| templates/cms_form_number.tpl | Switches counter label text to use the cms_form_charactersleft translate key. |
| templates/cms_form_location.tpl | Removes an unused/legacy field template variant. |
| templates/cms_footer.tpl | Loads jquery.simplyCountable.js and bumps magic.js cache-buster version. |
| include/locations_edit.php | Adds server-side max-length validation and sets maxlength + tooltip on the Label field. |
| db/migrations/20260725095602_widen_locations_label_column.php | Migration to widen locations.label to 50 chars (with down migration back to 20). |
| cypress/e2e/1_feature_tests/6_1_Warehouse_Locations.js | Adds Cypress coverage for boundary cases (50 accepted, 51 rejected). |
| assets/js/magic.js | Adds jQuery Validate maxlength rules for all [data-max-count] fields to block submit when over limit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
locations.label varchar(20)), which MySQL silently truncated with no feedback.locations.labeltovarchar(50)via phinx migration, and adds matching client-side (jQuery Validate, blocks submit) and server-side (mb_strlencheck, throws a clear exception) validation, plus a tooltip explaining why the limit exists (mobile navigation speed for warehouse/distribution teams).data-max-countcharacter counter's jQuery plugin (jquery.simplyCountable.js) existed in the repo but was never loaded on any page — no field had ever used themaxlengthoption before, so this was silently dead code.cms_form_charactersleft.Test plan
locations.labelisvarchar(50)viaSHOW COLUMNScypress/e2e/1_feature_tests/6_1_Warehouse_Locations.jscovering both cases; ran it against the live app in a real (headless) browser — both pass